Allow a custom equals parameter for observable collections#970
Allow a custom equals parameter for observable collections#970amondnet wants to merge 3 commits intomobxjs:mainfrom
Conversation
👷 Deploy request for mobx pending review.Visit the deploys page to approve it
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #970 +/- ##
==========================================
- Coverage 99.00% 98.38% -0.62%
==========================================
Files 57 57
Lines 2005 2043 +38
==========================================
+ Hits 1985 2010 +25
- Misses 20 33 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
amondnet
force-pushed
the
add-equals-observable-collection
branch
from
076d509 to
851e078
Compare
amondnet
force-pushed
the
add-equals-observable-collection
branch
from
851e078 to
d13d88d
Compare
amondnet
force-pushed
the
add-equals-observable-collection
branch
from
b1f29d8 to
fda5217
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR allows custom equality functions to be specified for MobX observable collections (ObservableList, ObservableMap, and ObservableSet). This enables developers to define custom comparison logic for determining when values are considered equal, which can optimize change detection and support custom equality semantics.
- Adds
EqualityComparer<T>? equalsparameter to all observable collection constructors - Implements custom equality logic in value comparison operations for all three collection types
- Updates
castmethods to preserve custom equality functions with optional overrides
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mobx/test/observable_collections_equals_test.dart | Comprehensive test suite covering custom equals functionality across all collection types |
| mobx/pubspec.yaml | Version bump from 2.5.0 to 2.6.0 for minor feature addition |
| mobx/lib/version.dart | Updates version constant to match pubspec |
| mobx/lib/src/api/observable_collections/observable_set.dart | Implements custom equals support for ObservableSet with updated constructors and equality logic |
| mobx/lib/src/api/observable_collections/observable_map.dart | Adds custom equals parameter for value comparisons in ObservableMap |
| mobx/lib/src/api/observable_collections/observable_list.dart | Integrates custom equals functionality into ObservableList operations |
| mobx/lib/src/api/observable_collections.dart | Adds required imports for collection package and utils |
| mobx/CHANGELOG.md | Documents the new custom equals feature in version 2.6.0 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| T? _findValue(Object? value) { | ||
| if (value is T?) { |
There was a problem hiding this comment.
The type check value is T? is incorrect for non-nullable types T. When T is non-nullable (e.g., String), T? becomes String?, but value with type Object? cannot be checked against T? directly. This should use value is T to check if value can be cast to the non-nullable type T.
| if (value is T?) { | |
| if (value is T) { |
| _list.cast<R>(), | ||
| equals ?? | ||
| (_equals != null | ||
| ? (R? a, R? b) => _equals!(a as T?, b as T?) |
There was a problem hiding this comment.
Unsafe casting from R? to T? without type checking. If R and T are incompatible types, this will cause a runtime exception. Should add type compatibility checks or use a safer casting approach.
| ? (R? a, R? b) => _equals!(a as T?, b as T?) | |
| ? (R? a, R? b) { | |
| if (a is T? && b is T?) { | |
| return _equals!(a, b); | |
| } | |
| return false; | |
| } |
| equals ?? | ||
| (_equals == null | ||
| ? null | ||
| : (RV? a, RV? b) => _equals!(a as V?, b as V?))); |
There was a problem hiding this comment.
Unsafe casting from RV? to V? without type checking. If RV and V are incompatible types, this will cause a runtime exception. Should add type compatibility checks or use a safer casting approach.
| : (RV? a, RV? b) => _equals!(a as V?, b as V?))); | |
| : (RV? a, RV? b) { | |
| if (a is V? && b is V?) { | |
| return _equals!(a, b); | |
| } | |
| // If types are incompatible, consider them not equal | |
| return false; | |
| })); |
| equals ?? | ||
| (_equals == null | ||
| ? null | ||
| : (R? a, R? b) => _equals!(a as T?, b as T?))); |
There was a problem hiding this comment.
Unsafe casting from R? to T? without type checking. If R and T are incompatible types, this will cause a runtime exception. Should add type compatibility checks or use a safer casting approach.
| : (R? a, R? b) => _equals!(a as T?, b as T?))); | |
| : (R? a, R? b) { | |
| if (a is! T || b is! T) return false; | |
| return _equals!(a as T?, b as T?); | |
| })); |
1 participant
Describe the changes proposed in this Pull Request.
Allow a custom equals parameter for observable collections, like observable stream, observable and computed .
#771
#482
If the PR fixes a specific issue, reference the issue with
Fixes #.Pull Request Checklist
pubspec.yamlis updated.major/minor/patch/patch-count, depending on the complexity of changemelos run set_versioncommand from the root directory